Fixing self imports for typeReference nodes#61107
Fixing self imports for typeReference nodes#61107navya9singh wants to merge 1 commit intomicrosoft:mainfrom
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
| if (symbol.valueDeclaration && getSourceFileOfNode(symbol.valueDeclaration).path === sourceFile.path) { | ||
| existingLocals.add(symbol); | ||
| } | ||
| else if (!symbol.valueDeclaration && symbol.parent?.valueDeclaration && getSourceFileOfNode(symbol.parent.valueDeclaration).path === sourceFile.path) { |
There was a problem hiding this comment.
I don’t think this is right. I'm not sure we ever should have been using valueDeclaration at all.
There was a problem hiding this comment.
Why do you think using valueDeclaration is wrong? Also, another way I had thought of was to do something like this
if (symbol.declarations && some(symbol.declarations, d => getSourceFileOfNode(d).path === sourceFile.path)) { existingLocals.add(symbol); }
but this again would go into another loop and check for each declaration which might probably end up taking even more time. Another way could be to do something like findAllReferences. I'm not exactly clear on what happens there but possible some way of comparing text to see which symbols already exist in a file. What do you think?
There was a problem hiding this comment.
valueDeclaration only includes value-meaning declarations, and the failing test case here makes it clear why that’s not sufficient. I think looking at symbol.declarations is correct.
|
With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies). Next steps for PRs:
|
4 participants
This pr fixes #58363 and #59827.